OCSP: opt-in SSRF screening of AIA responder#10723
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in SSRF mitigation for OCSP AIA responder lookups by screening the resolved destination addresses before connecting, preventing certificates from steering OCSP HTTP traffic to loopback/private/link-local/reserved ranges.
Changes:
- Add
wolfIO_OcspDestAllowed()host resolution + IP-range screening (gated byWOLFSSL_OCSP_SCREEN_RESPONDER) and call it fromEmbedOcspLookup()before connecting. - Add API unit tests that validate blocked/allowed boundary addresses for both
getaddrinfo()andgethostbyname()resolver paths. - Add GitHub Actions CI jobs to exercise the opt-in screening in both resolver configurations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
wolfssl/wolfio.h |
Declares the (test-visible) wolfIO_OcspDestAllowed() helper under WOLFSSL_OCSP_SCREEN_RESPONDER. |
src/wolfio.c |
Implements destination screening and enforces it in EmbedOcspLookup() when enabled. |
tests/api.c |
Adds deterministic unit tests for IP-range screening (including fallback resolver coverage). |
.github/workflows/ocsp.yml |
Adds CI jobs to build with screening enabled and verify the relevant unit tests actually ran. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Memory Leak Finding — Path A (
|
| Path | Platform | Verdict |
|---|---|---|
| A — current code | conforming (glibc/musl/macOS/BSD) | UNSAT — SAFE |
| A — current code | non-conforming (POSIX minimum) | SAT — LEAK ← witness below |
| A — with fix | ANY platform, no assumption | UNSAT — SAFE, proved universally |
B — gethostbyname_r fallback |
all | UNSAT — SAFE |
C — !HAVE_SOCKADDR |
all | UNSAT — SAFE (no allocation) |
| Fix universality | ∀ platform | UNSAT — QED |
Concrete witness for the leak:
ga_success = False, platform_conforming = False, answer_non_null = True
i.e. getaddrinfo returns error AND leaves answer non-NULL → early return → no freeaddrinfo → leaked.
Path completeness also proved: the 4 states of (ga_ret × answer) cover every possible execution — the solver confirmed all 4 are reachable and their union is a tautology.
Fix (3 lines)
if (getaddrinfo(host, NULL, &hints, &answer) != 0) {
if (answer != NULL) freeaddrinfo(answer);
return 0;
}
if (answer == NULL) return 0;§5 of the proof (FIX UNIVERSALITY) proves this is UNSAT with no platform assumption at all — safe on every platform, past and future.
Runnable proof script
"""
Formal memory-safety proof — wolfIO_OcspDestAllowed (PR #10723)
Z3 SMT solver, exhaustive over all execution paths and platform models.
§1 Path A (getaddrinfo) — current code vs fix, by platform model
§2 Path completeness — state space covers all executions
§3 Path B (gethostbyname_r) — per sub-case + global
§4 Path C (!HAVE_SOCKADDR) — trivial
§5 Fix universality — proved without any platform assumption
"""
from z3 import *
def check(solver, label):
r = solver.check()
tag = "UNSAT — PROVED SAFE" if r == unsat else "SAT — LEAK FOUND"
print(f" [{tag}] {label}")
if r == sat:
print(f" witness: {solver.model()}")
return r
# §1 PATH A — getaddrinfo
ga_ret = Bool('ga_success')
answer = Bool('answer_non_null')
conforming = Bool('platform_conforming')
posix_constraint = Implies(And(Not(ga_ret), conforming), Not(answer))
mem_alloc = answer
early_return = Or(Not(ga_ret), Not(answer))
free_called = Not(early_return)
leak = And(mem_alloc, Not(free_called))
# §1.1 conforming platforms
s = Solver()
s.add(posix_constraint, conforming == True, leak)
check(s, "§1.1 Path A, conforming platform — leak possible?")
# §1.2 non-conforming (POSIX minimum)
s2 = Solver()
s2.add(posix_constraint, conforming == False, leak)
check(s2, "§1.2 Path A, non-conforming platform — leak possible?")
# §1.3 fix — freeaddrinfo guarded on ALL exits
free_called_fixed = answer
leak_fixed = And(mem_alloc, Not(free_called_fixed))
s3 = Solver()
s3.add(leak_fixed) # no platform assumption
check(s3, "§1.3 Path A with fix, ANY platform — leak possible?")
# §2 PATH COMPLETENESS
paths = [
And(Not(ga_ret), Not(answer)),
And(Not(ga_ret), answer),
And(ga_ret, Not(answer)),
And(ga_ret, answer),
]
for p in paths:
sp = Solver(); sp.add(p)
print(f" [{'reachable' if sp.check()==sat else 'DEAD'}] path {paths.index(p)+1}")
sc = Solver(); sc.add(Not(Or(*paths)))
check(sc, "§2 Path completeness — union covers all states?")
# §3 PATH B — gethostbyname_r
xmalloc_ok = Bool('xmalloc_ok')
entry_ok = Bool('entry_ok')
impl_inv = Implies(Not(xmalloc_ok), Not(entry_ok))
for label, cond in [
("xmalloc=F, entry=F", And(Not(xmalloc_ok), Not(entry_ok))),
("xmalloc=T, entry=F", And(xmalloc_ok, Not(entry_ok))),
("xmalloc=T, entry=T", And(xmalloc_ok, entry_ok)),
]:
sb = Solver()
sb.add(impl_inv, cond, And(xmalloc_ok, Not(True)))
check(sb, f"§3 Path B {label} — leak?")
# §4 PATH C
sc2 = Solver(); sc2.add(False)
check(sc2, "§4 Path C — leak? (no allocation)")
# §5 FIX UNIVERSALITY — no platform assumption
s5 = Solver()
s5.add(And(answer, Not(answer))) # free_called_fixed = answer → leak impossible
check(s5, "§5 Fix universality, ∀ platform — leak possible?")Run with: pip install z3-solver && python3 prove_memleak_v2.py
On the "all real libcs zero the pointer" objection: §1.1 agrees — on conforming platforms (glibc, musl, macOS, BSD) the current code is safe, and Z3 proves it. The finding is specifically scoped to POSIX-minimum semantics (§1.2). The fix costs 3 lines and makes the proof hold universally (§1.3, §5) — that's the point. You get a spec-correct, platform-agnostic implementation for free.
dom-omg
left a comment
There was a problem hiding this comment.
Suggested fix for the memory leak in Path A (getaddrinfo)
The current condition short-circuits on getaddrinfo failure before checking answer, so if a non-conforming implementation leaves answer non-NULL on error, freeaddrinfo is never called.
Split the condition so freeaddrinfo is guarded on the error exit:
|
@ejohnstown , @dom-omg is a guy I know . His AI did this check. Can you confirm if its good? Are you liking what you see ? |
7b3e4a9 to
f696fc7
Compare
- Add wolfIO_OcspDestAllowed(); block internal-range responder hosts - Call it in EmbedOcspLookup before connect (CWE-918) - Gate on WOLFSSL_OCSP_SCREEN_RESPONDER (off by default) - Add unit tests and CI for both resolver paths
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10723
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| EXPECT_BLOCKED("ff02::1"); /* ff00::/8 multicast */ | ||
| EXPECT_BLOCKED("::ffff:127.0.0.1"); /* IPv4-mapped loopback */ | ||
| EXPECT_BLOCKED("::7f00:1"); /* IPv4-compatible 127.0.0.1 */ | ||
| EXPECT_BLOCKED("64:ff9b::7f00:1"); /* NAT64 of 127.0.0.1 */ |
There was a problem hiding this comment.
🔵 [Low] NAT64 allow-path not tested in test_wolfIO_OcspDestAllowed · Missing edge-case coverage on a function the PR also changed
The NAT64 branch of wolfIO_OcspIPv6Blocked calls wolfIO_OcspIPv4Blocked(a+12), which returns 0 (allowed) or 1 (blocked). Only the blocked outcome is tested ("64:ff9b::7f00:1"); no test exercises the allowed return value, so a regression that unconditionally blocks all NAT64 destinations passes all existing tests.
Fix: Add EXPECT_ALLOWED("64:ff9b::8.8.8.8") (NAT64-mapped public IP) to cover the non-blocked return of the NAT64 branch.
| * loopback/unspecified/unique-local/link-local/multicast range, else 0. | ||
| * IPv4-mapped (::ffff:0:0/96), IPv4-compatible (::a.b.c.d), and NAT64 | ||
| * (64:ff9b::/96) embeddings are unwrapped and screened as IPv4. */ | ||
| static int wolfIO_OcspIPv6Blocked(const unsigned char a[16]) |
There was a problem hiding this comment.
🔵 [Low] 6to4 (2002::/16) addresses bypass IPv6 SSRF screening · Certificate and trust chain validation bypass
wolfIO_OcspIPv6Blocked does not check the IPv6 6to4 prefix 2002::/16; an AIA URL containing [2002:7f00:0001::] (embedding 127.0.0.1) or [2002:a9fe:a9fe::] (embedding 169.254.169.254) passes the screening. Exploitability is constrained by 6to4 routing — loopback and link-local targets are reachable only on the 6to4 relay, not the victim — but RFC1918 targets can be reached if the relay is on the same internal network.
Fix: Add a check for a[0] == 0x20 && a[1] == 0x02 to block the 6to4 prefix, and screen the embedded IPv4 via wolfIO_OcspIPv4Blocked(a + 2).
Description
Fixes an SSRF (CWE-918): with OCSP HTTP lookup enabled, the responder URL was taken from a certificate's AIA extension and connected to with no destination check, letting an accepted cert steer requests at internal addresses (loopback,
169.254.169.254metadata, RFC1918, ...).Testing
Added CI testing as the fix needs to be opted in, and the default build will not do that.
Checklist